Skip to content

postinst: avoid false positives when scanning logs for errors#973

Open
AshwinUjjwal wants to merge 1 commit intoni:nilrt/master/scarthgapfrom
AshwinUjjwal:dev/fix-postinst-log-grep
Open

postinst: avoid false positives when scanning logs for errors#973
AshwinUjjwal wants to merge 1 commit intoni:nilrt/master/scarthgapfrom
AshwinUjjwal:dev/fix-postinst-log-grep

Conversation

@AshwinUjjwal
Copy link
Copy Markdown

@AshwinUjjwal AshwinUjjwal commented Mar 18, 2026

Summary of Changes

Replace deprecated egrep usage with grep -E and improve the log matching pattern to avoid false positives.

Previously, the pattern 'warn|error|fatal|fail' matched substrings such as 'str_error_r.o' and triggered failures due to the egrep deprecation warning.

Updated the command to use:
grep -Eiw 'warn(ing)?|error|fatal|fail'

This ensures only whole words are matched and includes both 'warn' and 'warning', reducing incorrect test failures.

Justification

AB#3429565

Testing

TODO: Detail what testing has been done to ensure this submission meets requirements.

  • I validated this change by running the test script on target(RTOS-8862): /usr/lib/run-postinsts/ptest/test-run-postinsts-log.sh
image

Note

This change should also be cherry‑picked into the dist-next and the relevant release branches.

Procedure

@AshwinUjjwal AshwinUjjwal marked this pull request as ready for review March 23, 2026 06:23
Copy link
Copy Markdown
Contributor

@rajendra-desai-ni rajendra-desai-ni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good, but since the change is just in the test and not in the actual postinst script, it is better to update the commit title to postinst-test: or something instead of postinst:

Also, I guess this commit needs to be cherry-picked to dist-next and release branches. Can you update the same in the PR description?

@rajendra-desai-ni rajendra-desai-ni requested a review from a team March 23, 2026 15:36
@AshwinUjjwal AshwinUjjwal force-pushed the dev/fix-postinst-log-grep branch from 632617f to 0023700 Compare March 23, 2026 18:01
# only present on first boot
if [ -e "$LOGFILE" ]; then
if egrep -i 'warn|error|fatal|fail' "$LOGFILE"; then
if grep -Eiw 'warn(ing)?|error|fatal|fail' "$LOGFILE"; then
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about strings like errors, failed, failure, etc? I think it's better to have false positives rather than false negatives.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the log scanning pattern to include additional keyword variations such as err, errored, errors, failed, and failure to improve coverage. Since log outputs are not fixed, included as many relevant keyword forms as possible to minimize false negatives.

Attempted to remove -w (using -Ei) for broader matching, but it introduced false positives from file names such as /lib/modules/.../str_error_r.o. Therefore, retained -Eiw to ensure meaningful matches .

Replace deprecated egrep usage with grep -E and improve the log
matching pattern to avoid false positives.

Previously, the pattern 'warn|error|fatal|fail' matched substrings
such as 'str_error_r.o' and triggered failures due to the egrep
deprecation warning.

Updated the command to use:
grep -Eiw 'warn(ing)?|error|fatal|fail'

This ensures only whole words are matched and includes both 'warn'
and 'warning', reducing incorrect test failures.

Signed-off-by: AshwinUjjwal <ashwinujjwal.bharti@emerson.com>
@AshwinUjjwal AshwinUjjwal force-pushed the dev/fix-postinst-log-grep branch from 0023700 to 99a2413 Compare March 25, 2026 18:07
@AshwinUjjwal AshwinUjjwal requested a review from chaitu236 March 26, 2026 16:46
Copy link
Copy Markdown
Contributor

@chaitu236 chaitu236 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. But we won't be cherry-picking this into release branch. And we should wait until release is complete to cherry-pick into -next branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants